Skip to content

[harmony] Bug 1851421: Use /a with \d to ensure ASCII digit matches#169

Merged
justdave merged 4 commits into
mainfrom
bug-1851421-harmony
Jul 11, 2026
Merged

[harmony] Bug 1851421: Use /a with \d to ensure ASCII digit matches#169
justdave merged 4 commits into
mainfrom
bug-1851421-harmony

Conversation

@justdave

@justdave justdave commented Jul 9, 2026

Copy link
Copy Markdown
Member

Details

Perl 5.14 started treating Unicode variants of digits as part of the \d character class, but added a /a modifier to indicate you really only want ASCII matches. Pretty much everywhere Bugzilla uses it, we explicitly only want ASCII digits.

This is a very comprehensive patch attacking every \d regexp in Bugzilla.

This patch was written by GPT-5.3-Codex, and manually reviewed by me. Most of the changes it made make sense to me (and I removed the few that didn't, which included a few minor code correctness cleanups that I liked but were out-of-scope for this patch) and it found several places I certainly would have missed had I done it manually.

It had the following to say about the changes it made:


This patch performs a broad Perl 5.14 compatibility hardening pass for regex digit handling across the Bugzilla codebase by making digit matches ASCII-explicit where needed (primarily by adding /a or using (?a:...)), so numeric IDs, dates, routes, and parser patterns no longer depend on Unicode digit semantics; the work spans core modules, CGI/scripts, extensions, and QA tests, and multiple files under extensions and qa.


This depends on #168 landing first (which is also included in this branch), and will be much easier to review after that lands and main gets merged into this PR.

Additional info

Test Plan

?? This certainly needs testing. I'm not quite sure how. A comprehensive test would be time consuming and hard to accomplish without lots of random data. If we want to do thorough testing for this it's probably better to push it off and not include it in the Bugzilla 5.9.2 release.

@justdave justdave requested review from dylanwh and mrenvoize July 9, 2026 13:37

@mrenvoize mrenvoize left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love to see a regression test for this, but I'm not going to treat that as a blocker.

You did miss updating the perl version in cpanfile though, could we get a tiny follow-up for that please.

Comment thread process_bug.cgi
Comment thread Bugzilla/Comment.pm
mrenvoize
mrenvoize previously approved these changes Jul 11, 2026

@mrenvoize mrenvoize left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks for the feedback. Approved now 😁

@justdave justdave merged commit 1ece9a5 into main Jul 11, 2026
8 checks passed
@justdave justdave deleted the bug-1851421-harmony branch July 11, 2026 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants